-
Notifications
You must be signed in to change notification settings - Fork 7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Clarification for only using feature IDs #40
Clarification for only using feature IDs #40
Conversation
Based on a PR comment, added an explanation and example of using feature IDs without property tables.
@@ -247,7 +247,22 @@ Each feature ID definition may include only a single source, so the following ar | |||
- `featureId.offset` and `featureId.repeat` (for a Feature ID Attribute) | |||
- `featureId.index` (for a Feature ID Texture) | |||
|
|||
Every `propertyTables` index must have an associated `featureIds` definition, but feature IDs may be defined without a property table. The `propertyTables` entry at index `i` corresponds to the `featureIds` entry at the same index. As a result, the length of the `featureIds` array must be greater than or equal to the length of the `propertyTables` array. Each (`featureId`, `propertyTable`) pair must be unique, but individual feature IDs and property tables may be repeated within a primitive or node. | |||
Feature IDs may be assigned to primitives or nodes, but do not have to be associated with a property table. This is useful when the feature IDs are supposed to be resolved externally by an application. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this could be worded a little clearly, and I think we can add another use case (using feature IDs for styling.)
Here's one possible wording:
Feature IDs may be assigned to primitives or nodes, but do not have to be associated with a property table. This is useful when the feature IDs are supposed to be resolved externally by an application. | |
Feature IDs may be assigned to primitives or nodes without being associated with a property table. This is useful when an application retrieves properties from an external resource (e.g. a database or REST API). This can also be used for identifying features in a mesh for styling in an application, even if no properties are stored. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh another thing, sets of feature IDs may be stored in primitives or nodes in the schema, but conceptually feature IDs themselves are only ever assigned to vertices, GPU instances or texels. it would be good to read through this section and make sure we're consistent with our wording.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hm... in thinking about this some more, "sets" of feature IDs could get a little ambiguous, as all the FEATURE_ID_n
semantics together could also be referred to as a "set of feature IDs"...
When in doubt, it might be better to be explicit and say "featureIds
entries" or something like that?
I think since this example is mainly about showing how to use feature ids without a property table the implicit feature ids might be a bit of a distraction and we should show a more common example using accessors (like |
I'll have to re-read the respective sections to figure out how the feedback might relate to an "established vocabulary", and then think about a sensible solution. If anybody has a specific suggestion about how things should be, I'd integrate that, but for now, I assume that this has low priority, or might be sorted out via some internal discussion. |
@ptrgags - I like this change. Should I create a PR, with a section, an example and a diagram? |
Coming back to this after a while, @ptrgags
If I understood that correctly, then the main point was to check whether we say "feature IDs are stored in a mesh primitive" at some point, and change that to something like "feature IDs for the vertices are stored in the mesh primitive". (It may not even be necessary to talk about "sets" here). But one could still argue whether they are really stored there, or only defined (or so). This also refers to the suggested change:
But "storing properties" may be a suitable short-hand form for (precise but clumsy) spec-parlance like ~"having non-zero-length An aside: If I had wrapped my head about the details of all documents, and all discussions that happened in issue threads, I'd probably have a clear mental model for the vocabularly, when it comes to "defining properties", "storing properties", "storing property values", "storing property definitions" and so on. I think there are differences, and I think they matter, but maybe they are less important than I thought.
Depending on whether this should be extended to a whole section, I can either extend this PR, or you can make a suggestion. For the diagram (or rather, an image) : We considered to have a somewhat more "generic" image in the overview that shows this case. Right now, the overview shows a table, but this could be replaced by some black box... But there may be ways of representing that in a more suitable form when it refers to a database in particular. |
@javagl - Nice. That works. How about a DB icon and something that looks like a DB table ? |
WIth that table, the diagram is again more similar to the original one (before I draw that 'black box' over the table). But I can create a "nice(r)" version of that diagram that can be used in that section, regardless of how far the text is extended or worded exactly. |
The section "Specifying Feature IDs" now has two subsections, "Resolving Feature IDs with Property Tables" and "Resolving Feature IDs Externally". Direct preview: https://github.com/javagl/glTF/tree/ext-mesh-features-only-feature-ids/extensions/2.0/Vendor/EXT_mesh_features#specifying-feature-ids
On the one hand, I agree that the details for the implicit ID in the "Example" may appear to be distracting. On the other hand, just having an example
does not convey much information, and might require a few words as well, and depending on the level of detail, may also be distracting. One could just say: "This defines the attribute that strores the IDs", or provide some context and say "The |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this section is a great addition, thanks! A few suggested wording changes attached.
Co-authored-by: Don McCurdy <dm@donmccurdy.com>
Co-authored-by: Don McCurdy <dm@donmccurdy.com>
Co-authored-by: Don McCurdy <dm@donmccurdy.com>
Co-authored-by: Don McCurdy <dm@donmccurdy.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two minor comments left, then this looks good to me! The external resources section is not normative presumably, so we can always expand it later if needed.
Co-authored-by: Don McCurdy <dm@donmccurdy.com>
This looks good to me, thanks @javagl! |
Clarification for only using feature IDs
Based on a PR comment, added an explanation and example of using feature IDs without property tables.
(See KhronosGroup#2082 (comment) )